Skip to content

Add Sql Server tests and improve logging#8086

Merged
davidfowl merged 1 commit intomainfrom
sebros/sqldbtests
Mar 15, 2025
Merged

Add Sql Server tests and improve logging#8086
davidfowl merged 1 commit intomainfrom
sebros/sqldbtests

Conversation

@sebastienros
Copy link
Contributor

Description

Added more tests for AddDatabase (taken from the Postgres work).
Changed database creation logging failure to use the resource logging.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

Copilot AI review requested due to automatic review settings March 15, 2025 00:55
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 15, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces new functional tests for SQL Server integration and improves the logging mechanism during database creation.

  • Adds tests for resilient database creation and multiple database scenarios.
  • Updates logging to use ResourceLoggerService to provide better contextual logging.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/Aspire.Hosting.SqlServer.Tests/SqlServerFunctionalTests.cs Adds functional tests covering resilient and multi-database creation scenarios.
src/Aspire.Hosting.SqlServer/SqlServerBuilderExtensions.cs Updates logging to use ResourceLoggerService for enhanced resource context logging.
Comments suppressed due to low confidence (1)

src/Aspire.Hosting.SqlServer/SqlServerBuilderExtensions.cs:262

  • Ensure that 'sqlDatabase.Parent' reliably returns a valid resource context for logging; if there is any chance it could be null, it may lead to a runtime exception.
var logger = serviceProvider.GetRequiredService<ResourceLoggerService>().GetLogger(sqlDatabase.Parent);

@davidfowl davidfowl merged commit f951020 into main Mar 15, 2025
163 checks passed
@davidfowl davidfowl deleted the sebros/sqldbtests branch March 15, 2025 03:19
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2025
@RussKie RussKie added area-integrations Issues pertaining to Aspire Integrations packages and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-integrations Issues pertaining to Aspire Integrations packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants